-
-
Notifications
You must be signed in to change notification settings - Fork 552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XWIKI-20546: Log CSS exception details #3518
Conversation
In XWikiWikiModel, if parsing CSS exception occurs include the exception in SLF4J call to get the full stack trace. This should help investigation of [XWIKI-20546](https://jira.xwiki.org/browse/XWIKI-20546) issue.
@@ -312,7 +312,7 @@ private String getImageDimension(String dimension, Map<String, String> imagePara | |||
value = sd.getPropertyValue(dimension); | |||
} catch (Exception e) { | |||
// Ignore the style parameter but log a warning to let the user know. | |||
this.logger.warn("Failed to parse CSS style [{}]", style); | |||
this.logger.warn("Failed to parse CSS style [{}]", style, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tend to only log a org.apache.commons.lang3.exception.ExceptionUtils.getRootCauseMessage(e)
in the case of a warning (a printed exception is pretty huge in the log, so we do that only for errors).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tmortagne for clarifying this coding rule.
Actually what I'm might do is deploying this custom version only in our development environment, reproduce the issue described in XWIKI-20546 and add the log in the Jira ticket comments.
I'm closing this pull request. Thanks for taking the time to look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I feel that adding at least the root cause message would be an improvement in any case, it's definitely not great to not have any kind of clue of what was wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm reopening the pull request this time logging only the root cause.
Also I took the time to read https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/ and properly configure my IDE.
In XWikiWikiModel, if parsing CSS exception occurs include the root cause of the exception in the log message. This should help investigation of [XWIKI-20546](https://jira.xwiki.org/browse/XWIKI-20546) issue.
Thanks a lot for your time @amottier ! That being said, my understanding is that it's not really a fix for https://jira.xwiki.org/browse/XWIKI-20546 and that we need to fix what cause this failure in the first place before closing the issue, right ? |
Yes you are totally right @tmortagne, main purpose of this pull request is to get appropriate level of information to be able to further analyze the issue. |
Jira URL
https://jira.xwiki.org/browse/XWIKI-20546
Changes
Description
Clarifications
CSSOMParser
.Executed Tests
mvn -f xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/pom.xml clean install
with Java 17 in Linux environment.Expected merging strategy